Skip to content

Perf: Throttle parallel typechecks in Find All References - #20128

Open
xperiandri wants to merge 2 commits into
dotnet:mainfrom
xperiandri:fix-find-references-throttle
Open

Perf: Throttle parallel typechecks in Find All References#20128
xperiandri wants to merge 2 commits into
dotnet:mainfrom
xperiandri:fix-find-references-throttle

Conversation

@xperiandri

@xperiandri xperiandri commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #20127

Problem

Project.FindFSharpReferencesAsync launched one CancellableTask per document in the project simultaneously via CancellableTask.whenAll, causing an unbounded number of parallel typechecks (one per document) at once during Find All References / Rename.

Fix

  • Added CancellableTask.whenAllThrottled maxDegreeOfParallelism in CancellableTasks.fs, using a SemaphoreSlim to cap concurrency while preserving cancellation semantics.
  • Applied it in Project.FindFSharpReferencesAsync (WorkspaceExtensions.fs), capping concurrency to max 1 Environment.ProcessorCount.

@xperiandri
xperiandri marked this pull request as ready for review August 3, 2026 19:32
@xperiandri
xperiandri requested a review from a team as a code owner August 3, 2026 19:33
@github-actions github-actions Bot added the AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files label Aug 3, 2026
T-Gro
T-Gro approved these changes Aug 4, 2026
View reviewed changes

@T-Gro T-Gro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI review (@expert-reviewer): no significant issues found. Please verify independently.

Reviewed the throttling implementation for correctness:

  • use semaphore combined with return! Task.WhenAll(tasks) correctly awaits within the use scope, avoiding the common SemaphoreSlim use-after-dispose pitfall (this would be a bug with return instead of return!).
  • semaphore.WaitAsync(ct) is placed before the try, so Release() runs only when a permit was actually acquired — no risk of over-release, and cancelled waiters correctly skip the release.
  • The expensive start ct task runs only after acquiring a permit, so concurrency is genuinely capped.
  • max 1 Environment.ProcessorCount guarantees a valid (>= 1) semaphore count.

Cancellation and exception-aggregation semantics match the existing whenAll. LGTM.

@github-project-automation github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Aug 4, 2026
@T-Gro
T-Gro self-requested a review August 4, 2026 09:11
@T-Gro T-Gro added the AI-reviewed PR reviewed by AI review council label Aug 4, 2026
@xperiandri
xperiandri force-pushed the fix-find-references-throttle branch from 149109e to c5582ba Compare August 5, 2026 00:26
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

❗ Release notes required

You can open this PR in browser to add release notes: open in github.dev

@xperiandri,

Caution

No release notes found for the changed paths (see table below).

Please make sure to add an entry with an informative description of the change as well as link to this pull request, issue and language suggestion if applicable. Release notes for this repository are based on Keep A Changelog format.

The following format is recommended for this repository:

`* . (PR #XXXXX)`

See examples in the files, listed in the table below or in th full documentation at https://fsharp.github.io/fsharp-compiler-docs/release-notes/About.html.

If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request.

Change path Release notes path Description
`vsintegration/src` docs/release-notes/.VisualStudio/18.vNext.md No release notes found or release notes format is not correct

@xperiandri
xperiandri force-pushed the fix-find-references-throttle branch from c5582ba to a86b21f Compare August 5, 2026 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@T-Gro T-Gro Awaiting requested review from T-Gro

Assignees

No one assigned

Labels

AI-reviewed PR reviewed by AI review council AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files

Projects

F# Compiler and Tooling
Status: In Progress

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Perf: Throttle parallel typechecks in Find All References

2 participants

@xperiandri @T-Gro